Skip to content

OLS-2882 Align spec structure and fix health issues#2000

Open
xrajesh wants to merge 2 commits into
openshift:mainfrom
xrajesh:xav/spec-first-update
Open

OLS-2882 Align spec structure and fix health issues#2000
xrajesh wants to merge 2 commits into
openshift:mainfrom
xrajesh:xav/spec-first-update

Conversation

@xrajesh
Copy link
Copy Markdown
Contributor

@xrajesh xrajesh commented May 29, 2026

Summary

  • Restructure .ai/spec/README.md with Structure table, Cross-Reference table, and standard Conventions section
  • Absorb what/README.md and how/README.md into the main spec README and remove them
  • Create ARCHITECTURE.md with Mermaid diagrams (system context, component architecture, data flow)
  • Add ## Specs pointer to AGENTS.md
  • Fix spec health issues: remove stale CloseButton.tsx/Modal.tsx refs, add missing pageContext.ts/validation.ts/CSS files to module map, remove completed tickets from Planned Changes

Test plan

  • Verify all spec file links resolve correctly
  • Verify Mermaid diagrams render in GitHub markdown preview
  • Confirm no behavioral spec content was changed (only structural alignment)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Restructured core project specifications with enhanced index organization and cross-reference mapping between behavioral and architectural guidance.
    • Added comprehensive architecture documentation covering system design, component interactions, state management, and documented key technical decisions.
    • Updated implementation roadmap for attachments and tools capabilities.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@openshift-ci openshift-ci Bot requested review from joshuawilson and kyoto May 29, 2026 21:21
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 29, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kyoto for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9a3624ff-0e6a-45c1-a5a6-42fe57ae076c

📥 Commits

Reviewing files that changed from the base of the PR and between 25579e5 and 73a0b94.

📒 Files selected for processing (1)
  • .ai/spec/how/project-structure.md
✅ Files skipped from review due to trivial changes (1)
  • .ai/spec/how/project-structure.md

📝 Walkthrough

Walkthrough

This PR consolidates documentation and specifications for the OpenShift LightSpeed console plugin. It reorganizes the spec system index, expands architecture documentation with diagrams and design decisions, updates feature planning roadmaps, and adds contributor guidance linking to the new specification structure.

Changes

Documentation and Specification System

Layer / File(s) Summary
Spec Index and Organization
.ai/spec/README.md, .ai/spec/how/project-structure.md
.ai/spec/README.md reorganized as a structured index with What/How sections, cross-reference matrix between spec categories, and updated conventions for spec authoring. .ai/spec/how/project-structure.md updated to document pageContext.ts (model-key resolution) and validation.ts (alert hashing), adjust component paths, and introduce a dedicated Component CSS subsection.
Feature Planning Updates
.ai/spec/what/attachments.md, .ai/spec/what/tools.md
Planned Changes sections updated: attachments.md replaces completed work with new ACM attachment features (ApplicationSet, policy violations, cluster info), and tools.md replaces completed item with new entries for tool UI extensibility, MCP App support, and tool info display.
Architecture Documentation
ARCHITECTURE.md
New comprehensive architecture document covering plugin responsibilities (UI-only), system context and component diagrams, SSE query lifecycle with throttled Redux updates, human-in-the-loop tool approval flow, MCP iframe sandboxing via postMessage JSON-RPC, Immutable.js Redux state slices, proxied API endpoints, extension points, technology stack, and key architectural decisions.
Contributor Guidance
AGENTS.md
Added top-level "Specs" section directing contributors to project specifications in .ai/spec/ and establishing .ai/spec/README.md as the entry point.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • openshift/lightspeed-console#2006: Both PRs modify AGENTS.md to add/extend contributor guidance pointing to .ai/spec/ with overlap in spec documentation direction.

Suggested labels

lgtm, approved

Suggested reviewers

  • joshuawilson
  • syedriko

Poem

🐰 Specs are now neat, organized with care,
Architecture docs floating everywhere,
What and How sections, a roadmap so clear,
Contributor welcome: the path's right here!
LightSpeed shines bright with this knowledge to share.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'OLS-2882 Align spec structure and fix health issues' accurately reflects the main changes: restructuring spec files, consolidating README files, and addressing spec health issues like stale references and missing documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch xav/spec-first-update

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.ai/spec/how/project-structure.md (1)

66-80: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix duplicate heading and mislabeled CSS section.

Line 74 repeats the same heading text, and Lines 70-72 list src/components/*.css under a src/assets/ heading. This makes the module map ambiguous and can trigger markdown lint noise.

Suggested doc fix
-### `src/assets/` -- Static assets
+### `src/components/` -- Component CSS assets

 | Path | Purpose |
 |---|---|
 | `src/components/general-page.css` | Styles for the GeneralPage chat interface |
 | `src/components/mcp-app.css` | Styles for MCP App card and iframe container |
 | `src/components/popover.css` | Styles for the Popover modal and floating button |

 ### `src/assets/` -- Static assets

As per coding guidelines: “All specifications live in .ai/spec/. Start with .ai/spec/README.md for project overview, reading order, and structure guide.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.ai/spec/how/project-structure.md around lines 66 - 80, The markdown has a
duplicated "src/assets/" heading and the components CSS entries are
miscategorized; change the first heading (the one above the CSS table) from
"src/assets/" to "src/components/" (or remove the duplicate and rename
accordingly), leave the second "src/assets/" section for logo/user asset rows
only, and ensure the `src/components/*.css` entries (GeneralPage, mcp-app,
popover) appear under the corrected "src/components/" heading so the module map
and linting are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In @.ai/spec/how/project-structure.md:
- Around line 66-80: The markdown has a duplicated "src/assets/" heading and the
components CSS entries are miscategorized; change the first heading (the one
above the CSS table) from "src/assets/" to "src/components/" (or remove the
duplicate and rename accordingly), leave the second "src/assets/" section for
logo/user asset rows only, and ensure the `src/components/*.css` entries
(GeneralPage, mcp-app, popover) appear under the corrected "src/components/"
heading so the module map and linting are consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8c057fc8-6a26-4007-9886-59e5044dd735

📥 Commits

Reviewing files that changed from the base of the PR and between 136c5b9 and 25579e5.

📒 Files selected for processing (9)
  • .ai/spec/README.md
  • .ai/spec/health-report.md
  • .ai/spec/how/README.md
  • .ai/spec/how/project-structure.md
  • .ai/spec/what/README.md
  • .ai/spec/what/attachments.md
  • .ai/spec/what/tools.md
  • AGENTS.md
  • ARCHITECTURE.md
💤 Files with no reviewable changes (4)
  • .ai/spec/how/README.md
  • .ai/spec/what/tools.md
  • .ai/spec/what/attachments.md
  • .ai/spec/what/README.md

Comment thread .ai/spec/health-report.md Outdated
@@ -0,0 +1,35 @@
# Spec health report
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this file should not be part of the PR, but these details could be included in the commit message / PR description instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyoto thank you . Fixed , Please take a look

Remove health-report.md from the PR per reviewer feedback —
details belong in commit message/PR description, not a committed
file. Fix mislabeled CSS section heading in project-structure.md
(was duplicating src/assets/, now correctly labeled src/components/).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2026
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jun 2, 2026

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants